feat(ui): add configurable StreamBackButton unread count#2816
feat(ui): add configurable StreamBackButton unread count#2816VelikovPetar wants to merge 1 commit into
StreamBackButton unread count#2816Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesUnread badge migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Header
participant StreamBackButton
participant StreamUnreadIndicator
participant ClientState
Header->>StreamBackButton: configure unreadCount
StreamBackButton->>StreamUnreadIndicator: select total or channel mode
StreamUnreadIndicator->>ClientState: read unread streams
ClientState-->>StreamUnreadIndicator: provide unread counts
StreamUnreadIndicator-->>StreamBackButton: render badge
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
StreamBackButton unread count
| /// | ||
| /// Set [excludeCid] to omit a channel's unread messages from the total - | ||
| /// for example, the currently open channel. | ||
| const factory StreamBackButtonUnreadCount.total({String? excludeCid}) = _TotalUnreadCount; |
There was a problem hiding this comment.
Not completely sure about the naming here, let me know if you think we should use something different (.unreadMessages()perhaps, to serve as a forecast for the potentially upcoming .unreadChannels())
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_chat_flutter/example/lib/main.dart`:
- Around line 228-232: Update the unread-count selection in the back-button
setup to use the total count excluding the current channel rather than
StreamBackButtonUnreadCount.channel(cid). Preserve the existing fallback
behavior when no channel CID is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4e9d622-7b75-4baf-801c-03086903330e
📒 Files selected for processing (11)
docs/docs_screenshots/test/localization/localization_rtl_test.dartmigrations/redesign/headers_and_icons.mdpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/example/lib/main.dartpackages/stream_chat_flutter/lib/src/channel/channel_header.dartpackages/stream_chat_flutter/lib/src/indicators/unread_indicator.dartpackages/stream_chat_flutter/lib/src/misc/back_button.dartpackages/stream_chat_flutter/lib/src/misc/thread_header.dartpackages/stream_chat_flutter/test/src/channel/channel_header_test.dartpackages/stream_chat_flutter/test/src/indicators/unread_indicator_test.dartpackages/stream_chat_flutter/test/src/misc/back_button_test.dart
| // Show the current channel's own unread count on the back button. | ||
| final unreadCount = switch (StreamChannel.of(context).channel.cid) { | ||
| final cid? => StreamBackButtonUnreadCount.channel(cid), | ||
| _ => const StreamBackButtonUnreadCount.total(), | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Show the total unread count excluding the current channel.
Configuring the back button to show the current channel's unread count (.channel(cid)) results in a suboptimal UX, as the count will typically be 0 while the user is actively viewing the channel.
Since navigating back from the ChannelPage takes the user to the channel list, it's generally preferable to show the total unread count excluding the current channel. This also accurately demonstrates the new default behavior of StreamChannelHeader introduced in this PR.
💡 Proposed fix
- // Show the current channel's own unread count on the back button.
- final unreadCount = switch (StreamChannel.of(context).channel.cid) {
- final cid? => StreamBackButtonUnreadCount.channel(cid),
- _ => const StreamBackButtonUnreadCount.total(),
- };
+ // Show the total unread count across other channels on the back button.
+ final unreadCount = StreamBackButtonUnreadCount.total(
+ excludeCid: StreamChannel.of(context).channel.cid,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Show the current channel's own unread count on the back button. | |
| final unreadCount = switch (StreamChannel.of(context).channel.cid) { | |
| final cid? => StreamBackButtonUnreadCount.channel(cid), | |
| _ => const StreamBackButtonUnreadCount.total(), | |
| }; | |
| // Show the total unread count across other channels on the back button. | |
| final unreadCount = StreamBackButtonUnreadCount.total( | |
| excludeCid: StreamChannel.of(context).channel.cid, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/stream_chat_flutter/example/lib/main.dart` around lines 228 - 232,
Update the unread-count selection in the back-button setup to use the total
count excluding the current channel rather than
StreamBackButtonUnreadCount.channel(cid). Preserve the existing fallback
behavior when no channel CID is available.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2816 +/- ##
==========================================
+ Coverage 71.28% 71.30% +0.02%
==========================================
Files 430 430
Lines 26930 26951 +21
==========================================
+ Hits 19196 19217 +21
Misses 7734 7734 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| final String? channelId; | ||
|
|
||
| /// The unread count configuration for the back button. | ||
| final StreamBackButtonUnreadCount? unreadCount; |
There was a problem hiding this comment.
Should we instead just ask for a Widget/StreamUnreadIndicator here directly? It will make it more composable and the api cleaner.
There was a problem hiding this comment.
Good idea, perhaps passing a pure Widget would be the most future-proof. Not sure if we would be opening the API a bit too much, but since it is a pattern already used in the SDK, I think we should be OK. I will draft a solution using a Widget instead of StreamBackButtonUnreadCount.
There was a problem hiding this comment.
There is a small caveat with this approach: If we rework it to pass a Widget? unreadIndicator, passing a StreamUnreadIndicator complicates the setup a bit because currently, its placement is done by nesting a child:
StreamUnreadIndicator(
offset: .zero,
excludeCid: excludeCid,
child: button, // <--- StreamBackButton
)This would mean that when building a StreamBackButton with a StreamUnreadIndicator, We will have to pass child: null in the indicator, and handle the positioning inside the StreamBackButton, something along the lines of:
if (_effectiveUnreadIndicator case final ind?) {
button = Stack(
clipBehavior: Clip.none,
children: [
button,
Positioned.fill(
child: FittedBox(
fit: BoxFit.none,
alignment: AlignmentDirectional.topEnd,
child: ind, // <---- StreamUnreadIndicator WITHOUT child
),
),
],
);
}Should be doable, but in my opinion, it makes the API pretty confusing. I wanted to bring this up, before committing to this solution.
| this.semanticLabel, | ||
| }) : _unreadType = _UnreadChannels(cid: cid); | ||
| }) : _unreadType = _UnreadChannels(cid: cid), | ||
| excludeCid = null; |
There was a problem hiding this comment.
I think we can support excludeCid here too. wdyt?
There was a problem hiding this comment.
In theory yes, should be feasible. I decided to not do it now because it wasn't part of the initial bug report. However I have one concern: In that case, we could in theory pass both cid and excludeCid to the _UnreadChannels config -> This can become a bit confusing, but if we prioritize the setup in the following way:
- If
cid != null-> show that specific channel unread count - If
excludeCid != null-> all unread channels - 1 (if excluded channel has unread) - Else -> all unread channels
(Ideally we would have different factory methods for channels vs currentChannel, but currently the channels factory handles both cases, so I think introducing new API might be confusing).
What do you think about this?
There was a problem hiding this comment.
lets skip it until we have a usecase
Submit a pull request
Linear: FLU-557
Github Issue: #
CLA
Description of the pull request
The
StreamChannelHeaderback-button unread badge counted the currently open channel in its total, so it never reflected "unread elsewhere".StreamBackButtonnow takes anunreadCountconfiguration via the newStreamBackButtonUnreadCount:.total({excludeCid})— total unread across channels, optionally excluding one channel..channel(cid)— a specific channel's unread count.StreamChannelHeadernow uses.total(excludeCid: channel.cid), so its back-button badge shows the unread messages waiting in other channels.StreamUnreadIndicatorgained an opt-inexcludeCid(bare usage is unchanged).showUnreadCountandchannelIdonStreamBackButtonare deprecated but still functional — fully backwards compatible.Test instructions: open a channel that has unread messages while other channels also have unread. The header back-button badge shows
total − thisChannel; going back to the channel list, the badge shows the full total. Covered by unit tests inback_button_test.dartandunread_indicator_test.dart(incl. the deprecated paths).Screenshots / Videos
unreads-before.mov
unreads-after.mov
Summary by CodeRabbit
New Features
Bug Fixes
Documentation